-
Notifications
You must be signed in to change notification settings - Fork 16
feat: adds support for BIP39 mnemonics #577
Conversation
WalkthroughThe changes introduced in the Changes
Poem
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- go.mod
Files selected for processing (2)
- cmd/blobstream/keys/evm/evm.go (6 hunks)
- cmd/blobstream/keys/evm/evm_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- cmd/blobstream/keys/evm/evm_test.go
Additional comments: 4
cmd/blobstream/keys/evm/evm.go (4)
1-18: The new imports look fine. Ensure that all the imported packages are being used in the code.
28-28: The constant
mnemonicEntropySize
is set to 256, which is the standard entropy size for a 24-word BIP39 mnemonic. This is correct.414-486: The
ImportMnemonic
command is correctly implemented. It prompts the user for a mnemonic, validates it, derives a private key from it, and imports the corresponding EVM address into the keystore. The command correctly handles errors and edge cases.639-662: The
MnemonicToPrivateKey
function correctly derives a private key from a BIP39 mnemonic. It uses the correct derivation path for Ethereum (m/44'/60'/0'/0
). The function correctly handles errors and edge cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cmd/blobstream/keys/evm/evm.go (6 hunks)
Additional comments: 2
cmd/blobstream/keys/evm/evm.go (2)
639-662: The
MnemonicToPrivateKey
function has been added to derive a private key from a mnemonic phrase. The function uses the Ledger derivation path to generate the first private key. This function looks good and follows the BIP39 standard for generating a private key from a mnemonic.414-488: The
ImportMnemonic
command has been added to import an EVM address from a 24-word BIP39 mnemonic phrase. The function asks for the mnemonic and passphrase from the user, derives the private key from the mnemonic, and imports the corresponding EVM address into the keystore. This function looks good and follows the BIP39 standard for importing a mnemonic.
cmd/blobstream/keys/evm/evm.go
Outdated
} | ||
} | ||
|
||
account, err := s.EVMKeyStore.NewAccount(passphrase) | ||
// read entropy seed straight from tmcrypto.Rand and convert to mnemonic | ||
entropySeed, err := bip39.NewEntropy(mnemonicEntropySize) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
mnemonic, err := bip39.NewMnemonic(entropySeed) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// get the private key | ||
ethPrivKey, err := MnemonicToPrivateKey(mnemonic, passphrase) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
account, err := s.EVMKeyStore.ImportECDSA(ethPrivKey, passphrase) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
logger.Info("account created successfully", "address", account.Address.String()) | ||
|
||
fmt.Println("\n\n**Important** write this mnemonic phrase in a safe place." + | ||
"\nIt is the only way to recover your account if you ever forget your password.") | ||
fmt.Printf("\n%s\n\n", mnemonic) | ||
return nil | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Add
function has been updated to generate a BIP39 mnemonic and derive a private key from it. The mnemonic is then printed to the console. This is a security risk as the mnemonic can be seen by anyone with access to the console logs. Consider removing the print statement or replacing it with a more secure method of delivering the mnemonic to the user.
- fmt.Printf("\n%s\n\n", mnemonic)
Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
} | |
} | |
account, err := s.EVMKeyStore.NewAccount(passphrase) | |
// read entropy seed straight from tmcrypto.Rand and convert to mnemonic | |
entropySeed, err := bip39.NewEntropy(mnemonicEntropySize) | |
if err != nil { | |
return err | |
} | |
mnemonic, err := bip39.NewMnemonic(entropySeed) | |
if err != nil { | |
return err | |
} | |
// get the private key | |
ethPrivKey, err := MnemonicToPrivateKey(mnemonic, passphrase) | |
if err != nil { | |
return err | |
} | |
account, err := s.EVMKeyStore.ImportECDSA(ethPrivKey, passphrase) | |
if err != nil { | |
return err | |
} | |
logger.Info("account created successfully", "address", account.Address.String()) | |
fmt.Println("\n\n**Important** write this mnemonic phrase in a safe place." + | |
"\nIt is the only way to recover your account if you ever forget your password.") | |
fmt.Printf("\n%s\n\n", mnemonic) | |
return nil | |
}, | |
} | |
} | |
} | |
// read entropy seed straight from tmcrypto.Rand and convert to mnemonic | |
entropySeed, err := bip39.NewEntropy(mnemonicEntropySize) | |
if err != nil { | |
return err | |
} | |
mnemonic, err := bip39.NewMnemonic(entropySeed) | |
if err != nil { | |
return err | |
} | |
// get the private key | |
ethPrivKey, err := MnemonicToPrivateKey(mnemonic, passphrase) | |
if err != nil { | |
return err | |
} | |
account, err := s.EVMKeyStore.ImportECDSA(ethPrivKey, passphrase) | |
if err != nil { | |
return err | |
} | |
logger.Info("account created successfully", "address", account.Address.String()) | |
fmt.Println("\n\n**Important** write this mnemonic phrase in a safe place." + | |
"\nIt is the only way to recover your account if you ever forget your password.") | |
// fmt.Printf("\n%s\n\n", mnemonic) | |
return nil | |
}, | |
} |
Codecov Report
@@ Coverage Diff @@
## main #577 +/- ##
==========================================
- Coverage 34.84% 27.96% -6.88%
==========================================
Files 27 29 +2
Lines 2230 2821 +591
==========================================
+ Hits 777 789 +12
- Misses 1360 1937 +577
- Partials 93 95 +2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cmd/blobstream/keys/evm/evm.go (6 hunks)
Additional comments: 5
cmd/blobstream/keys/evm/evm.go (5)
1-18: The added imports seem to be necessary for the new functionality. Ensure that all the imported packages are being used in the code.
28-28: The constant
mnemonicEntropySize
is set to 256, which is the correct size for a 24-word BIP39 mnemonic.92-126: The
Add
function has been modified to generate a mnemonic phrase and import an EVM account using the mnemonic. The function now generates a mnemonic, derives a private key from it, and imports the account into the keystore. The mnemonic is then printed to the console. This is a significant change and should be tested thoroughly.415-495: The
ImportMnemonic
function has been added. This function allows the user to import an EVM address from a 24-word BIP39 mnemonic phrase. The function reads the mnemonic from user input, validates it, and then derives a private key from it. The account is then imported into the keystore. This is a significant change and should be tested thoroughly.646-669: The
MnemonicToPrivateKey
function has been added. This function derives a private key from a given mnemonic and passphrase. It uses the Ledger derivation path to generate the first private key. This function is used in both theAdd
andImportMnemonic
functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note for reviewers]
It would be helpful to test the command while reviewing to see if the prompt messages are correct and intuitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- go.mod
Files selected for processing (2)
- cmd/blobstream/keys/evm/evm.go (7 hunks)
- cmd/blobstream/keys/evm/evm_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- cmd/blobstream/keys/evm/evm_test.go
Additional comments: 11
cmd/blobstream/keys/evm/evm.go (11)
1-18: The new imports are correctly added and are being used in the code. Ensure that these packages are added to the project dependencies.
25-30: The new constant
mnemonicEntropySize
is correctly defined and used in the code.86-93: The function
GetBIP39Passphrase()
is correctly called and its return value is checked for errors. The user is correctly informed about the purpose of the passphrase.101-120: The generation of the mnemonic and the derivation of the private key from it are correctly implemented. The error handling is also correctly done.
122-125: The import of the account is correctly done and the error is correctly handled.
129-136: The mnemonic is correctly printed to the console. The condition to check whether to print the passphrase is correctly implemented.
267-273: The
ImportMnemonic
command is correctly added to theimportCmd
command.435-504: The
ImportMnemonic
command is correctly implemented. The mnemonic is correctly read from the user input and its validity is checked. The passphrase is correctly read from the user input or from the command flags. The private key is correctly derived from the mnemonic and the account is correctly imported.637-656: The function
GetNewPassphrase()
is correctly implemented. The passphrase is correctly read from the user input and its confirmation is correctly checked. The loop correctly continues until the passphrase and its confirmation match.659-683: The function
GetBIP39Passphrase()
is correctly implemented. The passphrase is correctly read from the user input and its length is correctly checked. The passphrase confirmation is correctly checked. The loop correctly continues until the passphrase and its confirmation match or the passphrase length is correct.686-709: The function
MnemonicToPrivateKey()
is correctly implemented. The seed is correctly derived from the mnemonic and the passphrase. The master key is correctly computed from the seed. The private key is correctly derived from the master key. The Ethereum private key is correctly derived from the private key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- go.mod
Files selected for processing (2)
- cmd/blobstream/keys/evm/evm.go (7 hunks)
- cmd/blobstream/keys/evm/evm_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- cmd/blobstream/keys/evm/evm_test.go
Additional comments: 11
cmd/blobstream/keys/evm/evm.go (11)
1-18: The new imports seem to be used appropriately in the code. Ensure that these packages are added to the project dependencies.
28-28: The constant
mnemonicEntropySize
is set to 256, which is the standard entropy size for a 24-word BIP39 mnemonic. This is correct.86-93: The function
GetBIP39Passphrase()
is used to get the BIP39 passphrase from the user. The user is informed about the importance of the passphrase. This is a good practice for user interaction.104-120: The mnemonic is generated using the
bip39.NewEntropy
andbip39.NewMnemonic
functions. The private key is derived from the mnemonic using theMnemonicToPrivateKey
function. This is the correct way to generate a mnemonic and derive a private key from it.122-125: The private key is imported into the keystore using the
s.EVMKeyStore.ImportECDSA
function. This is the correct way to import a private key into a keystore.129-136: The mnemonic is displayed to the user with a warning about its importance. This is a good practice for user interaction.
267-271: The
ImportMnemonic
command is added to theimportCmd
command. This is the correct way to add a subcommand in Cobra.466-504: The
ImportMnemonic
command is implemented correctly. It gets the mnemonic from the user, validates it, gets the BIP39 passphrase, gets the store encryption passphrase, derives the private key from the mnemonic, and imports the private key into the keystore.637-655: The
GetNewPassphrase
function is updated to print a note about the store encryption passphrase. This is a good practice for user interaction.659-683: The
GetBIP39Passphrase
function is updated to limit the passphrase length to 100 characters and print a note about the BIP39 passphrase. This is a good practice for user interaction.686-709: The
MnemonicToPrivateKey
function is implemented correctly. It derives the master key from the mnemonic and passphrase using thebip39.NewSeedWithErrorChecking
function, and then derives the private key from the master key using thehd.DerivePrivateKeyForPath
function. This is the correct way to derive a private key from a mnemonic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- cmd/blobstream/keys/evm/evm.go (7 hunks)
Additional comments: 9
cmd/blobstream/keys/evm/evm.go (9)
1-18: The new imports seem to be used appropriately in the code. Ensure that these packages are added to the project dependencies.
28-28: The constant
mnemonicEntropySize
is set to 256, which is the standard entropy size for a 24-word BIP39 mnemonic. This is correct.86-93: The function
GetBIP39Passphrase()
is used to get the BIP39 passphrase from the user. Ensure that the passphrase is stored securely and not logged or exposed in any way.104-120: The
Add
command is modified to generate a mnemonic phrase and import the corresponding private key. This is a significant change and should be tested thoroughly.267-271: The
ImportMnemonic
command is added to import an EVM private key from a 24-word BIP39 mnemonic phrase. This is a significant change and should be tested thoroughly.429-506: The
ImportMnemonic
command is implemented correctly. It gets the mnemonic from the user, validates it, gets the BIP39 passphrase, and then uses theMnemonicToPrivateKey
function to derive the private key from the mnemonic. The private key is then imported into the keystore. Ensure that the mnemonic and passphrase are stored securely and not logged or exposed in any way.637-656: The
GetNewPassphrase
function is updated to improve user interaction. It now asks the user to enter the passphrase twice and checks if they match. This is a good practice to prevent typos.659-683: The
GetBIP39Passphrase
function is implemented correctly. It gets the BIP39 passphrase from the user, validates its length, and asks the user to enter it twice to check if they match. This is a good practice to prevent typos.686-709: The
MnemonicToPrivateKey
function is implemented correctly. It derives a private key from a BIP39 mnemonic using the default derivation path. This function should be tested thoroughly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Overview
Closes #495
Checklist
Summary by CodeRabbit